-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor Experiment endpoint and remove wtforms dependency #328
Refactor Experiment endpoint and remove wtforms dependency #328
Conversation
@alexb1200 The refactored experiment tests in #322 have been merged to |
2d1d542
to
1e1b5ab
Compare
Since this update changes the POST requests to no longer use forms, there are a couple places you'll need to update the Python client(s) (one being the deprecated client under dioptra/src/dioptra/client/_client.py Lines 581 to 584 in 1e1b5ab
The second place is this: dioptra/examples/scripts/client.py Lines 146 to 149 in 1e1b5ab
Both need the following change: response = requests.post(
self.experiment_endpoint,
- data=experiment_registration_form,
+ json=experiment_registration_form,
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a couple of small changes that are needed in schema.py
to ensure that the Swagger documentation looks right. In addition, a couple of small changes are needed in the Python client, see the comment I left that points out what to change.
Outside of the above, this looks good. The changes are passing on the new integration tests, so once these changes are in place, I think we'll be good to merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
Removed wtforms forms and corresponding models. Made minimal changes to unit tests.